Skip to content

Fix test-coverage workflow inputs for codecov/codecov-action@v6#634

Merged
yihui merged 1 commit into
Merck:mainfrom
jdblischak:fix-test-coverage
May 28, 2026
Merged

Fix test-coverage workflow inputs for codecov/codecov-action@v6#634
yihui merged 1 commit into
Merck:mainfrom
jdblischak:fix-test-coverage

Conversation

@jdblischak

Copy link
Copy Markdown
Collaborator

Follow-up to #626. Explanation in Merck/simtrial#364

@jdblischak jdblischak requested a review from yihui May 28, 2026 20:10
@jdblischak jdblischak self-assigned this May 28, 2026
@yihui yihui merged commit be20fab into Merck:main May 28, 2026
7 checks passed
@jdblischak jdblischak deleted the fix-test-coverage branch May 28, 2026 22:34
LittleBeannie added a commit that referenced this pull request Jun 24, 2026
* perf: replace object.size() with numhash() in prune_hash

object.size() walks the entire hash table structure on every
cache_fun() call, taking ~2ms per invocation. Since cache_fun is
called 15+ times per gs_design_ahr run (via expected_time/ahr and
gs_power_npe), this adds up to significant overhead.

Replace with numhash() which returns the entry count in O(1), and
use clrhash() for a simple eviction strategy when the limit is
exceeded.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* perf: replace dplyr full_join/select/arrange with base R in gs_design_npe

The output assembly in gs_design_npe used full_join (to merge H0 and
H1 probabilities), select, rename, and arrange from dplyr. Since
gs_design_npe is called once per gs_design_ahr and these operations
are on small data frames (6 rows), base R merge() and column
subsetting are much faster.

Combined with the gs_power_npe change, this yields ~50% overall
improvement for multi-analysis designs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* perf: replace dplyr operations with base R in gs_design_ahr output

Replace mutate, full_join, select, arrange, and filter operations
in the output assembly section of gs_design_ahr with equivalent base
R operations (direct column assignment, merge, column subsetting,
order).

This eliminates the dplyr overhead for the final output formatting
which previously involved multiple tibble round-trips on small data
frames.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* perf: remove dplyr from hot-path functions (expected_time, gs_info_ahr)

Replace select(-n) with base R column removal, and replace
mutate/transmute in the info_frac loop of gs_design_ahr with
direct column assignment. These functions are called repeatedly
during uniroot iterations, so even small per-call savings add up.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* perf: preserve tibble output type and fix row ordering

Exported functions gs_power_npe and gs_design_npe must return tibbles
for backward compatibility. Add tibble::as_tibble() at the return
point to convert the base R data.frame used for fast internal
computation back to the expected output type.

Also fix row ordering in gs_design_npe to maintain upper-before-lower
within each analysis (matching the original arrange(analysis) with
upper-first convention).

Refine prune_hash to use a 100-entry limit per function, giving
predictable memory bounds (each entry is typically a few KB, so
~100KB per cached function).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* perf: redesign cache pruning based on empirical measurement

Profiling revealed that object.size() overcounts gs_power_npe cache
entries by ~600x (reports 1.8 MB per entry when true incremental cost
is ~3 KB). This is because object.size() walks into shared namespace
environments of function arguments, counting the same gsDesign2
namespace (833 KB) and gsDesign namespace (75 KB) for every entry.

Changes:
- Remove object.size() from the pruning path (both slow and inaccurate)
- Only check entry count before insertions, not on cache hits
- Set max_entries = 1024, justified by:
  - True cost: ~3 KB (gs_power_npe) to ~5 KB (ahr) per entry
  - 1024 entries ≈ 3-5 MB real memory
  - Supports ~200 cached designs in a session
  - A single design creates only 5-28 entries

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* return data frame instead

* update the change point in the calculation of expected accrual (#628)

* Migrate GitHub Actions to Node 24 (#626)

* Use dependabot to auto-update GitHub Actions versions (#632)

https://docs.github.com/en/code-security/tutorials/secure-your-dependencies/dependabot-quickstart-guide
https://docs.github.com/en/code-security/how-tos/secure-your-supply-chain/secure-your-dependencies/configuring-dependabot-version-updates
https://docs.github.com/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file

* Fix test-coverage workflow inputs for codecov/codecov-action@v6 (#634)

* Bump yihui/actions from 1.1.0 to 1.1.2 (#633)

Bumps [yihui/actions](https://github.com/yihui/actions) from 1.1.0 to 1.1.2.
- [Release notes](https://github.com/yihui/actions/releases)
- [Commits](yihui/actions@v1.1.0...v1.1.2)

---
updated-dependencies:
- dependency-name: yihui/actions
  dependency-version: 1.1.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Unify test names for consistency (#639)

* Increase tolerance for test of gsDesign::gsSurv() vs gs_power_ahr() (#637)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Xie <xieyih@lctcvp7913.merck.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Yujie Zhao <43153957+LittleBeannie@users.noreply.github.com>
Co-authored-by: John Blischak <jdblischak@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants